feat: Clean cache command#1394
Conversation
d3xter666
force-pushed
the
feat-clean-cache
branch
2 times, most recently
from
83f2262 to
d52c4ad
Compare
RandomByte
force-pushed
the
feat/incremental-build-4
branch
from
7c59782 to
444977d
Compare
RandomByte
force-pushed
the
feat/incremental-build-4
branch
from
c2dc7b8 to
1041695
Compare
d3xter666
force-pushed
the
feat-clean-cache
branch
2 times, most recently
from
dc31834 to
f5def12
Compare
RandomByte
force-pushed
the
feat/incremental-build-4
branch
from
1041695 to
66296d5
Compare
d3xter666
force-pushed
the
feat-clean-cache
branch
2 times, most recently
from
77f7320 to
aa280da
Compare
d3xter666
force-pushed
the
feat-clean-cache
branch
from
34fd55f to
7473bd4
Compare
d3xter666
force-pushed
the
feat-clean-cache
branch
2 times, most recently
from
7b17cc0 to
569ff71
Compare
Provide common interface for cache cleanup, but distribute the real cleanup among the respective destinations
d3xter666
force-pushed
the
feat-clean-cache
branch
from
569ff71 to
36fd376
Compare
| @@ -0,0 +1,80 @@ | |||
| import path from "node:path"; | |||
There was a problem hiding this comment.
It seems odd that this file is placed next to the other files responsible for managing the framework packages but doesn't use any of them. I would expect better integration here, e.g. no hardcoded assumptions like the framework/ directory name as well as checks for existing lockfiles to prevent deleting files while a download is running
There was a problem hiding this comment.
I understand your point, but what botters me is that this might need a bit more of a refactoring.
Here's my rationale.
The AbstractInstaller is an abstract class that implements the lock logic. As the name suggests- it's an installer that is extended by the npm and maven installers.
Cache clanup, except from having in common the locks and paths, is a completelly different topic- it needs to clean the framework files. I have also seen that the framework folder is not configured in the AbstractInstaller, but is hardcoded in every installer.
The only clean option I forsee is to consolidate and reuse the locking logic and abstract the "framework" dir within the AbstractInstaller. Then somehow reuse this information in the cache cleaner.
There was a problem hiding this comment.
I have refactored the code, so that framework folder is reused accross classes and the locking is respecte.
Hopefully, this change addresses you comment: 8a53eb8
Let me know if you have other concerns on that matter
| * @param {string} dirPath Absolute path to directory | ||
| * @returns {Promise<number>} Total size in bytes | ||
| */ | ||
| async function getDirectorySize(dirPath) { |
There was a problem hiding this comment.
On my system this results in a different number than du:
~/.ui5
❯ du -sh framework
37G framework
❯ ui5 cache clean
The following items from cache will be removed:
• framework/ (31.9 GB)
• buildCache/v0_7 (70.7 MB)
Total: 32.0 GBIn my case, it also takes over a minute to calculate the size, which makes me wonder whether we should really do that or rather list the number of artifacts. Something like "this will delete 500 packages across 40 versions of UI5" would be much faster to calculate - not sure yet.
There was a problem hiding this comment.
Hmmm, that's fair!
Thanks for that feedback! I have not tested with that hughe amount of data!
I suppose the data difference comes from the way we calculate the conversion from bytes -> mb -> gb -> etc.
We do it by dividing to 1024 while in some systems they use 1000. I'm not sure whetehr this is the case here.
Maybe, if we check the bytes we can say whether it's the calculation base, or something within the node's fs.stats and du.
| export async function cleanCache(ui5DataDir) { | ||
| const frameworkDir = path.join(ui5DataDir, "framework"); | ||
| try { | ||
| const size = await getDirectorySize(frameworkDir); |
There was a problem hiding this comment.
Since it takes a long time to calculate the size of a large cache, could we skip calculating it again during the cleaning?
|
I think the usability of the command could be improved.
|
3 participants
The command completely cleans the cache by removing the cache files as well as cleaning up the SQLite records.
It does not wipe out the SQLite DB file(s)
JIRA: CPOUI5FOUNDATION-891